Skip to content

Conversation

@justonedev1
Copy link
Collaborator

Adding Cycle value to the metadata of the MonitorObject and QualityObject. These are stored in ccdb and used to pull correct version of object when using ForEachObject trigger.

During merging we use the maximum value of all encountered cycles. The same strategy is used when converting MOs to QOs.

Copy link
Collaborator

@knopers8 knopers8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I like the idea of adding "metadata" to interfaces instead of just "cycle" in particular. I have some change requests, but nothing serious.

Please also:

  • add tests for merging cycle numbers (testMonitorObjectCollection.cxx).
  • extend test_trigger_for_each_object to test cycle number
  • add support in TrendingTask

@justonedev1 justonedev1 force-pushed the QC-1086 branch 3 times, most recently from 839d349 to ed2cf61 Compare July 17, 2025 14:42
@justonedev1 justonedev1 requested a review from knopers8 July 17, 2025 14:45
Copy link
Collaborator

@knopers8 knopers8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I tried it myself, it works for me as well and does not seem to break anything else.

I still have a few pedantic requests, but after addressing those, the PR is good with me. Please update the documentation (doc/PostProcessing.md). In particular, mention that ForEachObject includes cycle number in the metadata of a returned trigger. Also, add a note that the example I want to run postprocessing on all already existing objects for a run works for both sync and async. Lastly, in doc/Framework.md / Monitor cycles, add a note that cycle number is stored as object metadata in QCDB

Copy link
Collaborator

@knopers8 knopers8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@knopers8 knopers8 enabled auto-merge (squash) July 22, 2025 13:31
@knopers8 knopers8 merged commit 891a040 into AliceO2Group:master Jul 22, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants